Implement Sentry instrumentation for performance and error tracking #470

cyrilS-dev · 2024-09-17T22:33:39Z

This Pull Request introduces Sentry integration for performance tracking and error monitoring.

Key changes:

FrameProcessor:

Add conditional Sentry import and initialization check
Implement custom instrumentation with Sentry spans in FrameProcessorMetrics to measure TTFB (Time To First Byte) and processing time when Sentry is available
Maintain existing metrics functionality with MetricsFrame regardless of Sentry availability

DeepgramSTTService:
1. Enable general metrics generation:
  - Implement can_generate_metrics method, returning True when VAD is enabled
  - This allows metrics to be collected and used by both Sentry and the metrics system in frame_processor.py
2. Integrate Sentry-compatible performance tracking:
  - Add start_ttfb_metrics and start_processing_metrics calls in the VAD speech detection handler
  - Implement stop_ttfb_metrics call when receiving transcripts
  - Add stop_processing_metrics for final transcripts
3. Enhance VAD support for metrics:
  - Add vad_enabled property to check VAD event availability
  - Implement VAD-based speech detection handler for precise metric timing

These changes allow for comprehensive performance monitoring and analysis using Sentry, providing valuable insights into STT, LLM and TTS services. The integration is designed to be non-intrusive, maintaining existing functionality while adding custom performance instrumentation.

Testing:

To validate the Sentry integration, the following steps were taken:

Sentry initialization was added to examples/simple-chatbot/demo.py:

sentry_sdk.init(
dsn="SENTRY_DSN",
integrations=[AsyncioIntegration()],
disabled_integrations=[HttpxIntegration()],
traces_sample_rate=1.0,
enable_tracing=True,
)

A Sentry transaction was created to encompass the main execution of the chatbot:

with sentry_sdk.start_transaction(op="pipecat", name="Pipecat"):
      await runner.run(task)

The vad_events parameter in the DeepgramSTTService configuration was set to True
The chatbot was run with this configuration, and the results were observed in the Sentry dashboard.
A screenshot of the Sentry performance monitoring results, demonstrating successful tracking of STT, LLM, and TTS services performance metrics :

These tests confirm that Sentry is correctly integrated and capable of monitoring both performance and errors across the pipeline components.

Notes:

It's crucial to initialize Sentry as early as possible in the application, and before from pipecat.processors.frame_processor :

To capture any potential errors that may occur during the imports
To ensure that sentry_sdk.is_initialized() returns the correct state when called in frame_processor.py.

Future Improvements:

Usage Metrics in Sentry Spans:
I plan to enhance the Sentry integration by adding "usage_metrics" to Sentry spans as tags.
This improvement will provide more detailed and contextual information about resource usage within each service, allowing for better analysis and optimization of our pipeline components.
Review TTS Processing Time Measurement:
I need to review and potentially refine the way we measure processing time for the Text-to-Speech (TTS) service.

This update add optional Sentry integration for performance tracking and error monitoring. Key changes include: - Add conditional Sentry import and initialization check - Implement Sentry spans in FrameProcessorMetrics to measure TTFB (Time To First Byte) and processing time when Sentry is available - Maintain existing metrics functionality with MetricsFrame regardless of Sentry availability

This commit enhances the DeepgramSTTService class to enable metrics generation for use with Sentry. Key changes include: 1. Enable general metrics generation: - Implement `can_generate_metrics` method, returning True when VAD is enabled - This allows metrics to be collected and used by both Sentry and the metrics system in frame_processor.py 2. Integrate Sentry-compatible performance tracking: - Add start_ttfb_metrics and start_processing_metrics calls in the VAD speech detection handler - Implement stop_ttfb_metrics call when receiving transcripts - Add stop_processing_metrics for final transcripts 3. Enhance VAD support for metrics: - Add `vad_enabled` property to check VAD event availability - Implement VAD-based speech detection handler for precise metric timing These changes enable detailed performance tracking via both Sentry and the general metrics system when VAD is active. This allows for better monitoring and analysis of the speech-to-text process, providing valuable insights through Sentry and any other metrics consumers in the pipeline.

aconchillo · 2024-09-17T23:42:27Z

This is fantastic. Thank you! I was thinking that in the future we might want to use something different than Sentry.

What if we create a FrameProcessorMetrics subclass (inside a new processors.metrics module), for example SentryMetrics. And then we pas that to the FrameProcessor constructor:

class FrameProcessor:

    def __init__(
            self,
            *,
            name: str | None = None,
            metrics: FrameProcessorMetrics = FrameProcessorMetrics(),
            loop: asyncio.AbstractEventLoop | None = None,
            **kwargs):

cyrilS-dev · 2024-09-18T00:06:20Z

Thank you for your feedback @aconchillo ! I really like your suggestion. It's definitely a more modular and scalable approach. I'll start working on this asap!

- Modified the __init__ method to accept a metrics parameter that is either FrameProcessorMetrics or one of its subclasses - Updated the metrics initialization to create an instance with the processor's name - Moved all FrameProcessorMetrics-related logic to a new processors\metrics\base.py file

1. Created a new metrics module in processors/metrics/ 2. Implemented FrameProcessorMetrics base class in base.py: 3. Implemented SentryMetrics class in sentry.py: - Inherits from FrameProcessorMetrics - Integrates with Sentry SDK for advanced metrics tracking - Implements Sentry-specific span creation and management for TTFB and processing metrics - Handles cases where Sentry is not available or initialized

cyrilS-dev · 2024-09-19T11:54:39Z

@aconchillo I've updated the PR to implement a flexible metrics system as you proposed. Here are the key changes:

Created a new processors/metrics/ directory with base.py containing the FrameProcessorMetrics base class and sentry.py with the SentryMetrics subclass.
Modified the FrameProcessor.__init__ method to accept a metrics parameter that can be either FrameProcessorMetrics or one of its subclasses

Now, you can choose the metrics implementation for each service. For example :

from pipecat.processors.metrics.sentry import SentryMetrics

tts = CartesiaTTSService(
api_key=os.getenv("CARTESIA_API_KEY"),
model_id="sonic-multilingual",
voice_id="65b25c5d-ff07-4687-a04c-da2f43ef6fa9",
language="fr",
metrics=SentryMetrics
)

If the metrics parameter is not specified, the default FrameProcessorMetrics will be used.

aconchillo · 2024-09-20T22:00:46Z

src/pipecat/processors/frame_processor.py

 class FrameProcessor:

    def __init__(
            self,
            *,
            name: str | None = None,
+            metrics: FrameProcessorMetrics = FrameProcessorMetrics,


I think this should be FrameProcessorMetrics() instead of passing a class. It's possible that other metrics implementation need additional arguments (api keys, user name, etc). If we hide the construction we would not be able to do that.

What we can do (since we need the name) is add a FrameProcessorMetrics.set_processor_name():

self._metrics = metrics metrics.set_processor_name(self.name)

With metrics: FrameProcessorMetrics = FrameProcessorMetrics(), the FrameProcessorMetrics() instance is created once when the FrameProcessor class is defined. As a result, every time a new FrameProcessor is created, it uses the same instance of FrameProcessorMetrics, and it's a problem. My idea was to use metrics: FrameProcessorMetrics | None = None and

self._metrics = metrics or FrameProcessorMetrics() self._metrics.set_processor_name(self.name)

to create a new instance of FrameProcessorMetrics inside the constructor if metrics is not provided. Let me know if i'm missing something, thanks.

Oh yes! You are totally right 😅 . Well, the important point was for people to be able to create instances from outside which we now can do! 😄

aconchillo · 2024-09-20T22:02:44Z

Looks great! There was a bigger metrics PR that got merged yesterday so unfortunately you will need to rebase your changes. Sorry :-(.

Btw, I think instead of base.py we can call it frame_processor_metrics.py we do that with other base classes as well.

cyrilS-dev · 2024-09-22T13:21:23Z

Looks great! There was a bigger metrics PR that got merged yesterday so unfortunately you will need to rebase your changes. Sorry :-(.

Btw, I think instead of base.py we can call it frame_processor_metrics.py we do that with other base classes as well.

done ✅

aconchillo · 2024-09-23T04:48:35Z

src/pipecat/processors/metrics/sentry.py

+    import sentry_sdk
+    sentry_available = sentry_sdk.is_initialized()
+    if not sentry_available:
+        logger.debug("Sentry SDK not initialized. Sentry features will be disabled.")


it seems this should be a warning.

aconchillo · 2024-09-23T04:48:42Z

src/pipecat/processors/metrics/sentry.py

+        logger.debug("Sentry SDK not initialized. Sentry features will be disabled.")
+except ImportError:
+    sentry_available = False
+    logger.debug("Sentry SDK not installed. Sentry features will be disabled.")


aconchillo · 2024-09-23T04:53:24Z

LGTM! I added a couple of minor comments. Before I merge, do you mind fixing linting issues? Just run autopep8. We will be moving to Ruff shortly, but for now that's what we are checking.

cyrilS-dev · 2024-09-23T10:03:13Z

LGTM! I added a couple of minor comments. Before I merge, do you mind fixing linting issues? Just run autopep8. We will be moving to Ruff shortly, but for now that's what we are checking.

Fixed! Thanks for the help @aconchillo

aconchillo · 2024-09-23T15:43:25Z

Merging. Thank you!

cyrilS-dev · 2024-09-23T15:52:29Z

thank you @aconchillo !
Just to understand, why the "Formatting lints" job failed?

aconchillo · 2024-09-23T15:54:00Z

thank you @aconchillo ! Just to understand, why the "Formatting lints" job failed?

I just saw... not sure. I'll take a look and fix. Btw, I'll enable vad_events by default otherwise we won't have metrics for Deepgram.

cyrilS-dev · 2024-09-23T16:03:15Z

thank you @aconchillo ! Just to understand, why the "Formatting lints" job failed?

I just saw... not sure. I'll take a look and fix. Btw, I'll enable vad_events by default otherwise we won't have metrics for Deepgram.

You are right, vad_events should be enabled by default. I initially left it disabled because it is currently used only for metrics. I'm working on a version where Deepgram's vad_events will emit UserStartedSpeakingFrame and UserStoppedSpeakingFrame when Silero VAD is not used. We can discuss this in the dedicated PR I will create.

aconchillo · 2024-09-23T17:44:57Z

thank you @aconchillo ! Just to understand, why the "Formatting lints" job failed?

It's because of this. c7ff79a

But I just merged the Ruff PR. autopep8 was doing very weird stuff.

…470) * feat: Add Sentry support in FrameProcessor This update add optional Sentry integration for performance tracking and error monitoring. Key changes include: - Add conditional Sentry import and initialization check - Implement Sentry spans in FrameProcessorMetrics to measure TTFB (Time To First Byte) and processing time when Sentry is available - Maintain existing metrics functionality with MetricsFrame regardless of Sentry availability * feat: Enable metrics in DeepgramSTTService for Sentry This commit enhances the DeepgramSTTService class to enable metrics generation for use with Sentry. Key changes include: 1. Enable general metrics generation: - Implement `can_generate_metrics` method, returning True when VAD is enabled - This allows metrics to be collected and used by both Sentry and the metrics system in frame_processor.py 2. Integrate Sentry-compatible performance tracking: - Add start_ttfb_metrics and start_processing_metrics calls in the VAD speech detection handler - Implement stop_ttfb_metrics call when receiving transcripts - Add stop_processing_metrics for final transcripts 3. Enhance VAD support for metrics: - Add `vad_enabled` property to check VAD event availability - Implement VAD-based speech detection handler for precise metric timing These changes enable detailed performance tracking via both Sentry and the general metrics system when VAD is active. This allows for better monitoring and analysis of the speech-to-text process, providing valuable insights through Sentry and any other metrics consumers in the pipeline. * Update frame_processor.py * Refactor to support flexible metrics implementation - Modified the __init__ method to accept a metrics parameter that is either FrameProcessorMetrics or one of its subclasses - Updated the metrics initialization to create an instance with the processor's name - Moved all FrameProcessorMetrics-related logic to a new processors\metrics\base.py file * Implement flexible metrics system with Sentry integration 1. Created a new metrics module in processors/metrics/ 2. Implemented FrameProcessorMetrics base class in base.py: 3. Implemented SentryMetrics class in sentry.py: - Inherits from FrameProcessorMetrics - Integrates with Sentry SDK for advanced metrics tracking - Implements Sentry-specific span creation and management for TTFB and processing metrics - Handles cases where Sentry is not available or initialized

fatwang2 · 2024-12-24T03:35:50Z

hey, I have read the doc about how to use sentry in pipecat, but I am still confused, can you share more examples about that?

fatwang2 · 2024-12-24T06:47:07Z

it worked on tts, but didn't work on llm

cyrilS-dev added 3 commits September 17, 2024 21:57

Update frame_processor.py

ad97e75

cyrilS-dev added 2 commits September 19, 2024 13:40

cyrilS-dev and others added 2 commits September 19, 2024 14:09

Add missing newline at end of file

bbbcf4f

Merge branch 'main' into sentry-impl

3c5483c

aconchillo reviewed Sep 20, 2024

View reviewed changes

cyrilS-dev added 2 commits September 21, 2024 22:48

Merge remote-tracking branch 'upstream/main' into sentry-impl

3b3f1e5

Refactor to Align with Merged PR pipecat-ai#474

cd13aff

aconchillo reviewed Sep 23, 2024

View reviewed changes

aconchillo approved these changes Sep 23, 2024

View reviewed changes

cyrilS-dev added 4 commits September 23, 2024 11:56

Format with autopep8 & adjust logger level

f7f4c95

Format with autopep8

a2adca1

Format with autopep8

4ef47b2

Format with autopep8

6d6d2cd

aconchillo merged commit dfa4ac8 into pipecat-ai:main Sep 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Sentry instrumentation for performance and error tracking #470

Implement Sentry instrumentation for performance and error tracking #470

cyrilS-dev commented Sep 17, 2024 •

edited

Loading

aconchillo commented Sep 17, 2024

cyrilS-dev commented Sep 18, 2024

cyrilS-dev commented Sep 19, 2024 •

edited

Loading

aconchillo Sep 20, 2024

cyrilS-dev Sep 21, 2024 •

edited

Loading

aconchillo Sep 23, 2024

aconchillo commented Sep 20, 2024

cyrilS-dev commented Sep 22, 2024

aconchillo Sep 23, 2024

cyrilS-dev Sep 23, 2024

aconchillo Sep 23, 2024

cyrilS-dev Sep 23, 2024

aconchillo commented Sep 23, 2024

cyrilS-dev commented Sep 23, 2024

aconchillo commented Sep 23, 2024

cyrilS-dev commented Sep 23, 2024

aconchillo commented Sep 23, 2024

cyrilS-dev commented Sep 23, 2024 •

edited

Loading

aconchillo commented Sep 23, 2024

fatwang2 commented Dec 24, 2024

fatwang2 commented Dec 24, 2024

Implement Sentry instrumentation for performance and error tracking #470

Implement Sentry instrumentation for performance and error tracking #470

Conversation

cyrilS-dev commented Sep 17, 2024 • edited Loading

aconchillo commented Sep 17, 2024

cyrilS-dev commented Sep 18, 2024

cyrilS-dev commented Sep 19, 2024 • edited Loading

aconchillo Sep 20, 2024

Choose a reason for hiding this comment

cyrilS-dev Sep 21, 2024 • edited Loading

Choose a reason for hiding this comment

aconchillo Sep 23, 2024

Choose a reason for hiding this comment

aconchillo commented Sep 20, 2024

cyrilS-dev commented Sep 22, 2024

aconchillo Sep 23, 2024

Choose a reason for hiding this comment

cyrilS-dev Sep 23, 2024

Choose a reason for hiding this comment

aconchillo Sep 23, 2024

Choose a reason for hiding this comment

cyrilS-dev Sep 23, 2024

Choose a reason for hiding this comment

aconchillo commented Sep 23, 2024

cyrilS-dev commented Sep 23, 2024

aconchillo commented Sep 23, 2024

cyrilS-dev commented Sep 23, 2024

aconchillo commented Sep 23, 2024

cyrilS-dev commented Sep 23, 2024 • edited Loading

aconchillo commented Sep 23, 2024

fatwang2 commented Dec 24, 2024

fatwang2 commented Dec 24, 2024

cyrilS-dev commented Sep 17, 2024 •

edited

Loading

cyrilS-dev commented Sep 19, 2024 •

edited

Loading

cyrilS-dev Sep 21, 2024 •

edited

Loading

cyrilS-dev commented Sep 23, 2024 •

edited

Loading